Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/add plugin bc test #2902

Open
wants to merge 17 commits into
base: alpha
Choose a base branch
from

Conversation

kwizer15
Copy link
Contributor

@kwizer15 kwizer15 commented Sep 17, 2024

Prévention des Breaking Change sur les signatures des méthodes de classes PHP.

Description

Permet de se prémunir de changements qui pourraient casser le code des plugins.
Vérifie les changements de visibilité, le typage, le nommage des méthodes en se basant sur un fichier généré grâce au code actuel.

Contient la PR sur l’ajout du .env et la réactivation des tests phpunit.

image

Suggested changelog entry

Add unit tests to prevent BC on PHP classes.

Related issues/external references

N/A

Types of changes

  • Bug fix (non-breaking change which fixes)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

$stmt->execute($_params);

$errorInfo = $stmt->errorInfo();
if ($errorInfo[0] != 0000) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug to fix here ?

if ($errorInfo[0] !== '00000') {

Quand on envoi un paramètre qui n’existe pas dans la requète, ca ne passe pas dedans.
Le vrai code 00000 (5 zéro) et est une chaine de caractère. Là on pourrait retirer 3 zéros, ca ferai la même chose.

(voir test_query_with_bad_parameter, qui correspond au comportement actuel, devrait probablement renvoyer une exception)

@pifou25
Copy link
Contributor

pifou25 commented Sep 29, 2024

Ici pour les workflow github on est dans un env de type docker on n'a pas besoin justement de ta PR sur le .env en mode php, il suffit d'utiliser les variables d'environnement pour générer le fichier common.config.php ... Tu pourrais séparer les 2 ?

J'ai tenté il y a qq mois de faire passer les tests phpunit existant mais pas réussi, avec un workflow spécifique (sur mon repo pour l'instant):
https://github.com/pifou25/jeedom-core/blob/feat/run_phpunit/.github/workflows/phpunit.yml

@kwizer15
Copy link
Contributor Author

Ici on ne génère aucun fichier justement, j'utilise directement les variables d'environnements.
Effectivement on pourrait le faire, on peut tout faire) mais ca rendrait le fichier github plus compliqué, le lancement des tests en local plus compliqué.

Et là tout fonctionne.

@kwizer15 kwizer15 force-pushed the feat/add-plugin-bc-test branch 2 times, most recently from e318377 to fb5fd52 Compare September 29, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants